-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat: added selection of entropy crate #3776
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: added selection of entropy crate #3776
Conversation
318f08a to
9634fb3
Compare
src/vmm/Cargo.toml
Outdated
| [features] | ||
| default = [] | ||
| aws-lc-rs = ["dep:aws-lc-rs"] | ||
| rand = ["dep:rand"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One of aws-lc-rs or rand is required. Currently if someone doesn't configure this properly they will get a non-obvious error.
You can add a build.rs like:
fn main() {
#[cfg(any(
all(feature="aws-lc-rs", feature="rand"), // If both are enabled
not(any(feature="aws-lc-rs", feature="rand")) // If neither are enabled
)]
compile_error!("Please enable the feature \"aws-lc-rs\" OR the feature \"rand\".");
}This should also be added to firecracker and devices.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I can add this. The problem though isn't that no features are enabled, but opposite. For some reason both of them are enabled at the same time and this causes problems.
|
I thought hat aws-lc-rs published a version that doesn't require the musl g++ chain 🤔 |
13af016 to
5a94c57
Compare
Added ability to select between `rand` and `aws-lc-rs` crates for entropy device. Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
5a94c57 to
9b7a1a3
Compare
|
Why do we need |
|
@bchalios Basically with |
You should be able to do that with |
|
I'd also like to have this feature to allow something other than aws-lc, although I didn't have any issue building it without musl. From the side of distro packaging, at least Fedora requires crypto libraries undergo additional review, and it might also require unbundling the C++ library from the Rust crate. Since |
Changes
Added ability to select between
randandaws-lc-rscrates for entropy device.If build with default command:
cargo buildeverything will be build withaws-lc-sys.If build with
cargo build --no-default-features --features randeverything will be build withrand.Reason
aws-lc-rscrate requires musl compatible c++ compiler which is not an easily obtainable dependencyWithout the musl c++ toolchain compiling Firecracker gives the error:
CMake Error at CMakeLists.txt:9 (enable_language): The CMAKE_C_COMPILER: musl-gcc is not a full path and was not found in the PATH. Tell CMake where to find the compiler by setting either the environment variable "CC" or the CMake cache entry CMAKE_C_COMPILER to the full path to the compiler, or to the compiler name if it is in the PATH.License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following
Developer Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md.PR Checklist
CHANGELOG.md.TODOs link to an issue.rust-vmm.